feat(devtools)!: reuse the built-in Terminals dock via ctx.terminals#1026
Open
antfubot wants to merge 2 commits into
Open
feat(devtools)!: reuse the built-in Terminals dock via ctx.terminals#1026antfubot wants to merge 2 commits into
antfubot wants to merge 2 commits into
Conversation
Retire Nuxt DevTools' bespoke @xterm terminal UI + RPC and surface terminal
sessions through the Vite DevTools terminals host (ctx.terminals), so they
render in the built-in Terminals dock.
Existing modules keep working: the devtools:terminal:* hooks are bridged onto
ctx.terminals as read-only registered sessions (module still owns the process
and streams output). A new opt-in interactive PTY path is added via a
devtools:terminal:spawn hook and the startDevToolsTerminal() helper.
Exit-driven cleanup for the internal module-install / analyze-build / npm-update
flows is preserved by broadcasting terminal exits from their startChildProcess
producers.
Adopts the devframe 0.7 docks.activate({ sessionId }) capability: the in-client
"reveal terminal" affordances (module install, build analyze, npm update) now
focus the built-in Terminals dock on the relevant session via a revealTerminal
RPC, instead of the removed Terminals tab.
Created with the help of an agent.
antfubot
force-pushed
the
feat/terminals-reuse-devframe
branch
from
July 21, 2026 08:59
badf065 to
4f421ba
Compare
Contributor
📝 WalkthroughWalkthroughTerminal support now bridges Nuxt terminal hooks to the built-in DevTools Terminals dock. New spawn options, hooks, RPC contracts, and a public helper support PTY or child-process sessions. Client terminal pages, xterm integration, and route-based navigation are removed; module and build workflows reveal terminals through RPC. Server-side analyze-build and npm operations broadcast process exit codes, while documentation and dependency configuration are updated. Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/devtools/src/server-rpc/npm.ts (1)
65-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winEnsure the terminal exit is broadcast even if the session rejects.
If
session.getResult()rejects (e.g., due to a failure to spawn the process), the exit is never broadcast. This can leave the client's UI in a perpetual "updating" state. Consider broadcasting a non-zero exit code in the.catchblock.♻️ Proposed fix to broadcast on error
// Surface the exit to the client so the update UI state can settle. void Promise.resolve(session.getResult()).then((result) => { broadcastTerminalExit(ctx, processId, result.exitCode) - }).catch(() => {}) + }).catch(() => { + broadcastTerminalExit(ctx, processId, 1) + }) return {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/devtools/src/server-rpc/npm.ts` around lines 65 - 78, Update the Promise chain around session.getResult() so its catch handler calls broadcastTerminalExit with processId and a non-zero exit code when the session rejects. Preserve the existing result.exitCode broadcast for successful sessions, ensuring every terminal outcome notifies the client.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/devtools/src/server-rpc/npm.ts`:
- Around line 65-78: Update the Promise chain around session.getResult() so its
catch handler calls broadcastTerminalExit with processId and a non-zero exit
code when the session rejects. Preserve the existing result.exitCode broadcast
for successful sessions, ensuring every terminal outcome notifies the client.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 008b91d4-8ba3-471e-8395-83c3a464aeff
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
docs/content/2.module/1.utils-kit.mdpackages/devtools-kit/src/_types/hooks.tspackages/devtools-kit/src/_types/rpc.tspackages/devtools-kit/src/_types/terminals.tspackages/devtools-kit/src/index.tspackages/devtools/client/components/ModuleItem.vuepackages/devtools/client/components/NpmVersionCheck.vuepackages/devtools/client/components/TerminalPage.vuepackages/devtools/client/components/TerminalView.vuepackages/devtools/client/composables/state-routes.tspackages/devtools/client/composables/state.tspackages/devtools/client/nuxt.config.tspackages/devtools/client/pages/modules/analyze-build.vuepackages/devtools/client/pages/modules/modules.vuepackages/devtools/client/pages/modules/terminals.vuepackages/devtools/client/setup/client-rpc.tspackages/devtools/package.jsonpackages/devtools/src/server-rpc/analyze-build.tspackages/devtools/src/server-rpc/index.tspackages/devtools/src/server-rpc/npm.tspackages/devtools/src/server-rpc/terminals.tsplaygrounds/module-starter/playground/nuxt.config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (9)
- packages/devtools/client/pages/modules/terminals.vue
- packages/devtools/client/components/TerminalPage.vue
- packages/devtools/client/composables/state.ts
- packages/devtools/client/components/TerminalView.vue
- packages/devtools/client/setup/client-rpc.ts
- packages/devtools/package.json
- packages/devtools/client/composables/state-routes.ts
- pnpm-workspace.yaml
- packages/devtools/client/nuxt.config.ts
…ions
`setupRPC()`'s `ctx.devtoolsKit` is a getter backed by a closure variable set
once the Vite DevTools kit connects. Both `setupRPC`'s own return statement
and `module-main.ts`'s destructuring spread `ctx` (`{ connectDevToolsKit,
...ctx }` / `const { ..., ...ctx } = setupRPC(...)`), and object spread reads
a getter's *current* value into a plain property on the new object — at that
point (synchronous module setup, before the kit has connected) the value is
always `undefined`. This permanently froze `ctx.devtoolsKit` to `undefined`
for every integration wired through `module-main.ts` (vscode, vue-devtools,
vite-inspect, vue-tracer, plugin-metrics, timeline), even long after the kit
actually connected.
Concretely, this made the VS Code Server launcher a silent no-op: its
`ctx.devtoolsKit?.terminals.startChildProcess(...)` call was always skipped
by the optional-chain, so clicking "Launch" never spawned `code-server` and
nothing showed up in the Terminals dock.
Fix: return/destructure `ctx` as a nested property instead of spreading it,
so the getter — and therefore the live connected kit — stays intact.
Verified against a real Nuxt dev server: pre-fix, `ctx.devtoolsKit` is
`undefined` at the VS Code integration's call site and no process spawns;
post-fix it resolves to the connected kit and `startChildProcess` returns a
real, tracked child-process session.
Created with the help of an agent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements plan
02-terminals-reusefrom the Vite DevTools integration series. Nuxt DevTools stops shipping its own@xtermterminal UI + RPC and instead surfaces terminal sessions through the devframe terminals host (ctx.terminals), so they render in Vite DevTools' built-in Terminals dock (which auto-hides when empty). The Nuxt "Terminals" tab is removed.Existing modules keep working via a compat shim, and a new opt-in interactive PTY path is added.
Rebased on
mainafter the Vite DevTools Kit0.4.2/ devframe0.7.5upgrade (#1029) and the Messages unification (#1025), and updated to adopt the new devframe capabilities.What changed
Server bridge (
server-rpc/terminals.ts, rewritten)devtools:terminal:*hooks ontoctx.terminalsinstead of maintaining its ownMap+ RPC:register→ a read-onlyDevframeTerminalSessioncarrying aReadableStream(registered viactx.terminals.register). Re-registration resets/clears in place, sostartSubprocess().clear()/restart()still work.write→controller.enqueue(data);exit→ close stream +update(status);remove→ dispose + remove the session.devtools:terminal:spawnhook →ctx.terminals.startChildProcess(output-only) orctx.terminals.startPtySession(interactive PTY,zigptywith graceful pipe fallback).devtools:readycallback.devframe 0.7 capability —
docks.activate({ sessionId })revealTerminal(id)RPC that callsctx.docks.activate('devframes_plugin_terminals', { sessionId })to focus the built-in Terminals dock on a specific session.Kit surface (
@nuxt/devtools-kit)SpawnTerminalOptions, thedevtools:terminal:spawnhook, astartDevToolsTerminal()helper (opt-in PTY), and therevealTerminalRPC type.getTerminals/getTerminalDetail/runTerminalAction+onTerminalDatafrom the RPC types (keptonTerminalExit).TerminalState/TerminalInfo/TerminalActionand thedevtools:terminal:*module-facing hooks are preserved.Internal producers — module-install / analyze-build / npm-update already spawn via
startChildProcess; they now broadcastonTerminalExiton process exit so their transient UI state (installing-modules, "Building…", update spinner) still clears. Exit-driven cleanup is not lost.Client — deleted the
terminals.vuetab,TerminalPage.vue,TerminalView.vue; removeduseTerminals,useCurrentTerminalId, and theonTerminalDatahandler. Removed the@xterm/*deps frompackage.json,pnpm-workspace.yaml, and the clientnuxt.config.tsbundling/optimizeDeps; refreshed the lockfile (the built-in@devframes/plugin-terminalsowns xterm now).Docs — documented
startDevToolsTerminal()and refreshed the "Terminals tab" references.Bug fix — VS Code Server launcher was a silent no-op. While validating this
change against a real dev server, found and fixed a pre-existing bug (from
#966, unrelated to this PR's changes) that broke every
module-main.tsintegration relying on
ctx.devtoolsKit(VS Code Server, Vue DevTools, ViteInspect, Vue Tracer, plugin-metrics, timeline):
setupRPC()'s returnstatement and
module-main.ts's destructuring both spreadctx(
{ connectDevToolsKit, ...ctx }), andctx.devtoolsKitis a getter — objectspread reads a getter's current value into a plain property, which at that
point (synchronous module setup, before the kit connects) is always
undefined. This permanently frozectx.devtoolsKittoundefined, so e.g.clicking "Launch" on the VS Code Server tab silently did nothing (its
ctx.devtoolsKit?.terminals.startChildProcess(...)call was always skipped bythe optional chain) — no process spawned, nothing in the Terminals dock.
Fixed by returning/destructuring
ctxas a nested property instead ofspreading it. Verified against a real dev server: pre-fix,
ctx.devtoolsKitis
undefinedat the call site and no process spawns; post-fix it resolvesto the connected kit and a real, tracked
code-serverchild process spawns.Notes / breaking changes
getTerminals/getTerminalDetail/runTerminalAction) and the@xtermUI are removed. Module-facing hooks (devtools:terminal:register/write/remove/exit) andstartSubprocess()continue to work via the shim.startSubprocess/getProcess()deprecation codes (NDT_DEP_0004/0001) already shipped; this PR swaps the underlying implementation rather than adding new codes.pnpm typecheckcurrently reports one pre-existing error in theclient/server/api/echo.post.tsfixture that is present onmain(surfaced by the deps: upgrade Vite DevTools kit to 0.4.2 #1029 dep bump), not introduced here.Verification
pnpm lint,pnpm build, andpnpm test:unitpass;pnpm typecheckis clean apart from the pre-existingecho.post.tsfixture error noted above.This PR was created with the help of an agent.